Fix GH-18120: Honor FILE_SKIP_EMPTY_LINES even when FILE_IGNORE_NEW_LINES is not set#19346
Fix GH-18120: Honor FILE_SKIP_EMPTY_LINES even when FILE_IGNORE_NEW_LINES is not set#19346alexandre-daubois wants to merge 3 commits intophp:masterfrom
FILE_SKIP_EMPTY_LINES even when FILE_IGNORE_NEW_LINES is not set#19346Conversation
0294ac1 to
feea545
Compare
feea545 to
c783b4b
Compare
c783b4b to
95211df
Compare
|
Friendly ping @DanielEScherzer @nielsdos as you participated on the issue 🙂 |
|
I'll play with this tomorrow or so. Note that this is fixing long standing behaviour, so targeting 8.5/master is likely more appropriate. |
|
^ what @nielsdos said about not targeting 8.3, but wearing my release manager hat I don't think 8.5 is appropriate |
<?php
$test_file = __DIR__ . "/file_skip_empty_lines.tmp";
$test_data = "\r";
file_put_contents($test_file, $test_data);
$lines = file($test_file, FILE_SKIP_EMPTY_LINES);
var_dump($lines);
$test_data = "\r\r";
file_put_contents($test_file, $test_data);
$lines = file($test_file, FILE_SKIP_EMPTY_LINES);
var_dump($lines);produces: which seems incorrect to me. Can you explain whether this is the desired behaviour? |
25718ef to
43d24d1
Compare
d8ff4fd to
daf2b51
Compare
|
@ndossche I addressed your concern and updated the test file as well. Also, I rebased on master as suggested. |
daf2b51 to
f0e3236
Compare
|
This still acts in a way I don't expect: $test_data = "\r\r\n\n";
file_put_contents($test_file, $test_data);
$lines = file($test_file, FILE_SKIP_EMPTY_LINES);
var_dump($lines);There are only empty lines in this test file, so the output should be an empty array. Am I misunderstanding something? Admittingly it's been a while since I looked at this. |
e392d00 to
369ec87
Compare
|
I took over the PR again, it fixes cases mentioned in the review while being lighter. @ndossche I added your latest test case to |
Fix #18120
Allows to use
FILE_SKIP_EMPTY_LINESwithoutFILE_IGNORE_NEW_LINES: